0.1c — amico-validate CLI + CI fast-tier schema gate (closes #17) - #30
Conversation
jack-champagne
left a comment
There was a problem hiding this comment.
CLI's correct and the gate genuinely fails red — I checked the bash -e errexit path, a malformed config (even a middle one) reds the step. Two changes before merge, both small (inline). Plus the manifest.toml refs here must move in lockstep with #28's run.toml rename.
Tests + Phase 1: two test-debt items vs #17 — the invalid-fixture corpus is one-class-per-file with no wrong-type case (only an in-test mutation covers it; AC7 wants the class matrix self-contained), and the linked amico-validate bin is never exercised in CI (inline). The bigger Phase-1 item: this gate validates only static committed artifacts. Phase 1's reference-prompt smoke corpus IS the real emit→validate gate, and this CI step is where it plugs in — wire one real amico-run → emit run-dir → validate lane here (or in #31/#18), not just configs, so the producer seam is actually covered before Phase 1 piles E2E on top. That's also the infra decision: the cloud solve-service (aws-infra) writes this same run-dir contract to S3, so the gate the cloud track inherits should have seen real emitted output, not just fixtures.
Don't overbuild past this — one ci.yml edit + a build-order fix, not more machinery.
| $V packages/extension/scripts/lab.toml.example --schema lab | ||
| $V packages/extension/demo/run/manifest.toml | ||
| $V packages/extension/demo/run/result.toml | ||
| $V packages/extension/demo/run/FINISHED |
There was a problem hiding this comment.
[important] the gate covers lab/manifest/result/FINISHED but not solvespec or catalog-entry — AC5 requires the non-filename schemas via --schema, and the DoD is "all five." Both fixtures are committed; add them:
| $V packages/extension/demo/run/FINISHED | |
| $V packages/extension/demo/run/FINISHED | |
| $V packages/schema/test/fixtures/valid/solvespec.toml --schema solvespec | |
| $V packages/schema/test/fixtures/valid/catalog-entry.toml --schema catalog-entry |
There was a problem hiding this comment.
Done — the gate now also validates solvespec + catalog-entry via --schema (the committed valid fixtures), so all five are covered (AC5 / DoD).
| run: | | ||
| V="node packages/schema/dist/amico-validate.js" | ||
| $V packages/extension/scripts/lab.toml.example --schema lab | ||
| $V packages/extension/demo/run/manifest.toml |
| "type": "module", | ||
| "main": "./src/index.ts", | ||
| "types": "./src/index.ts", | ||
| "bin": { "amico-validate": "./dist/amico-validate.js" }, |
There was a problem hiding this comment.
[important] this bin doesn't link on a clean install — dist/amico-validate.js is built by esbuild after pnpm install, so install warns Failed to create bin … ENOENT and the amico-validate command is never created. CI misses it because it runs the bundle path directly. For a slice whose deliverable is "ship the CLI," that's the entry point broken. Fix: a prepare script that builds on install, and have CI exercise the linked bin (pnpm exec amico-validate --help) so a broken entry point reds.
There was a problem hiding this comment.
Fixed in this push — the bin points at a committed launcher (packages/schema/launcher/amico-validate, mirroring amico-run) that exists at install time, so the bin links cleanly (the dist bundle it execs is built by pnpm build). CI now runs pnpm --filter amicode-v2 exec amico-validate --help (the bin links into the dependents, not the root) so a broken entry point reds.
| * --schema. The amico-validate CLI uses this for file-role resolution. */ | ||
| export function kindForFilename(filePath: string): SchemaKind | undefined { | ||
| const base = filePath.replace(/^.*[\\/]/, ""); | ||
| if (base === "manifest.toml") return "manifest"; |
There was a problem hiding this comment.
[rename] this manifest.toml (+ the SCHEMAS.manifest key, the schema filename, and the ci.yml gate line) must follow #28's run.toml rename together — miss one and basename resolution silently falls through to "cannot infer."
There was a problem hiding this comment.
Done — kindForFilename now resolves run.toml→"run", and the SCHEMAS key / schema filename / ci.yml gate / cli.test KINDS all follow #28s rename. Added a self-contained wrong-type fixture so the class matrix no longer leans on an in-test mutation.
5def952 to
ade5a6d
Compare
A user-facing validator over the shared @amicode/schema validate() core, plus a CI step that reds the PR on a schema-invalid config. No parallel validation logic. - src/cli.ts: `amico-validate <file> [--schema <kind>]`. Resolves file→schema by basename for the fixed-filename artifacts (manifest.toml/result.toml/lab.toml/ FINISHED via kindForFilename); --schema override for solvespec/catalog-entry or any non-standard name. Exit 0 valid / 64 invalid-or-usage (Q85). Field-precise stderr (offending key + JSON-pointer path). - bin amico-validate → dist/amico-validate.js (esbuild, shebang+chmod); ajv bundles. - test/fixtures/invalid/: one representative malformed file per kind (the shared negative corpus); valid corpus from 0.1a is the shared positive corpus. - test/cli.test.ts (8): valid→0 + invalid→64 across every kind, file-role resolution, FINISHED-by-basename, non-filename-needs-schema, field-precise stderr, usage errors → 64, --help → 0. - ci.yml: fast-tier `amico-validate` gate over the SHIPPED configs (lab.toml.example + demo/run manifest/result/FINISHED) after build. schema 32 / amico-run 47 / extension 63 green; CLI bundle 308kb. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…all five, wrong-type fixture, rename lockstep Address Jack's #30 review (rebased on #28's run.toml rename): - bin now points at a COMMITTED launcher (packages/schema/launcher/amico-validate, mirrors amico-run) instead of the un-built dist/amico-validate.js — so the `amico-validate` bin links on a clean install (was "Failed to create bin … ENOENT"; dist is built by `pnpm build`, absent at install time). CI now exercises the LINKED bin (`pnpm --filter amicode-v2 exec amico-validate --help`) so a broken entry reds. - CI gate validates ALL FIVE: added solvespec + catalog-entry via --schema (AC5 / DoD). - Self-contained wrong-type fixture (invalid/result-wrongtype.toml) + a cli.test case — the class matrix no longer relies on an in-test mutation (AC7). - Rename lockstep: index.ts kindForFilename + cli.ts usage + ci.yml gate + cli.test KINDS + invalid/run.toml all follow #28's manifest.toml→run.toml / "manifest"→"run". schema 34 / amico-run 47 / extension 64 (+1 skip) green; linked bin + gate verified locally. (The bigger real-emit→validate lane Jack flagged lands in #31, the round-trip slice.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
f8daf3f to
c8f9863
Compare
…nd-to-end (rebased on #28 rename) Address Jack's #31 review: - REAL producer lane (not just fixtures): scripts/producer_roundtrip.sh runs amico-run on a minimal non-Piccolo emitter (julia/emit_fixture.jl) so it produces a live run-dir (run.toml + FINISHED from the orchestrator, result.toml from the Julia emitter), then validates the FRESHLY-EMITTED artifacts with validate.jl. CI runs it (fast tier; the real Piccolo solve stays the slow/nightly extension). Scoped to what each side actually emits — the orchestrator writes run.toml/FINISHED, the Julia producer writes result.toml. - AC6 exercised, not asserted: scripts/ac6_drift_check.sh perturbs the SHARED schema, REBUILDS TS (it bakes schemas at build time — the gap Jack found), then asserts BOTH the TS bin AND validate.jl reject a previously-valid fixture, then reverts. Proven locally; wired into the schema-roundtrip CI job. runtests.jl's AC6 testset notes the build-vs- runtime asymmetry. - Rename lockstep: validate.jl + runtests.jl manifest.toml→run.toml, "manifest"→"run", run.schema.json; resolved the ci.yml conflict keeping #30's all-five gate + this job. schema 34 / amico-run 47 / extension 64 (+1 skip) / julia 22 green; producer + AC6 scripts pass locally. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jack-champagne
left a comment
There was a problem hiding this comment.
Re-reviewed c8f9863a. All three resolved:
- Wrong-type fixture self-contained —
result-wrongtype.toml(fidelity="high", a string where a float is required); AC7's class matrix no longer leans on an in-test mutation. - Linked bin exercised + gating in CI — the
fastjob runspnpm --filter amicode-v2 exec amico-validate --help(a broken bin entry reds) and the committed launcher validates all five schemas underbash -e. manifest.toml → run.tomlin lockstep with #28.
Approving. Retargeting to main + merging behind #29.
Stacked on #29 (0.1b). Do not merge.
A user-facing validator over the shared
@amicode/schemavalidate()core, plus a CI step that reds the PR on a schema-invalid config — no parallel validation logic.What's here
src/cli.ts:amico-validate <file> [--schema <kind>]. File→schema by basename for the fixed-filename artifacts (manifest.toml/result.toml/lab.toml/FINISHEDviakindForFilename);--schemaoverride forsolvespec/catalog-entryor any non-standard name. Exit 0 valid / 64 invalid-or-usage (Q85). Field-precise stderr (offending key + JSON-pointer path).bin amico-validate→dist/amico-validate.js(esbuild, shebang + chmod; ajv bundles).test/fixtures/invalid/: one representative malformed file per kind — the shared negative corpus (the 0.1a valid corpus is the shared positive corpus; one corpus, all surfaces).test/cli.test.ts(8): valid→0 + invalid→64 across every kind, file-role resolution, FINISHED-by-basename, non-filename-needs---schema, field-precise stderr, usage→64,--help→0.ci.yml: fast-tieramico-validategate over the shipped configs (lab.toml.example+ demomanifest/result/FINISHED) after build.Review
Adversarial code review: no must-fix / should-fix correctness. Verified empirically — every argv/edge path returns the right exit,
kindForFilenamedoesn't misresolve (result.toml.bak→ undefined), the CIbashword-split works (local zsh-only quirk), the gate runs after build, shebang +chmod +xpresent. One optional hardening nit (CI invokes the bundle directly rather than the linked bin) — non-blocking.Tests
schema 32 / amico-run 47 / extension 63 green; CLI bundle 308kb.
🤖 Generated with Claude Code